Skip to content

AgentServer Security fixes + Fix POH teleport utilities and Rs2Walker behavior in instances#1744

Merged
chsami merged 21 commits intomainfrom
development
Apr 15, 2026
Merged

AgentServer Security fixes + Fix POH teleport utilities and Rs2Walker behavior in instances#1744
chsami merged 21 commits intomainfrom
development

Conversation

@chsami
Copy link
Copy Markdown
Owner

@chsami chsami commented Apr 15, 2026

This pull request introduces significant improvements to the Microbot agent server's security and configuration, along with several bugfixes and enhancements for POH (Player-Owned House) pathfinding and tile/object detection. The most important changes are grouped below:

Agent Server Security and Configuration:

  • Added authentication to the agent server: all HTTP API requests now require a shared secret token (X-Agent-Token header), which is auto-generated and stored in both the plugin config and ~/.microbot/agent-token file. Token file permissions are set securely, and the file is deleted on shutdown. The plugin now enforces loopback-only access and disables cross-origin requests. [1] [2] [3] [4] [5] [6] [7] [8]
  • The agent server plugin is now disabled by default for improved safety.
  • Increased the required microbot dependency version to 2.5.0.

POH Pathfinding and Object Detection Fixes:

  • Fixed POH pathfinding: when inside a POH instance, the pathfinder now remaps the start location to the user-configured exit portal tile, ensuring POH teleports are considered and preventing walker loops. [1] [2]
  • Improved detection of POH exit portal: switched from using the player's template-mapped world location (which fails in instances) to using the tile-object cache, making detection reliable inside POH instances. [1] [2]
  • Added a static getter for the configured POH exit portal tile, making it accessible for other components.

Tile/Object Query Robustness:

  • Improved robustness of tile and game object queries: when the anchor world location cannot be mapped to a scene-local coordinate (common in POH instances), the code now falls back to the player's actual local location, ensuring queries succeed in all cases. [1] [2] [3]

Other Fixes:

  • Corrected the deposit box inventory item container component ID.

These changes collectively enhance the security, reliability, and user experience of the Microbot agent server and POH-related features.

Adam- and others added 17 commits April 10, 2026 22:43
…tances (#1741)

Any script that called Rs2Walker.walkTo(overworldPoint) while the player was
inside their POH hung forever, tight-looping "localpoint returned null". The
same root cause broke PohTeleports.isInHouse / usePortalNexus / useJewelleryBox
and the Shortest Path "Detect available POH teleports" button on any non-small
house.

Root cause: Rs2Player.getWorldLocation() returns the overworld template-mapped
tile (e.g. 1877,7052,1) inside a POH instance, not the player's real scene
coords. Every WorldPoint -> LocalPoint -> Tile lookup in the walker, shortest
path, poh, and gameobject utilities is fed that template tile, which isn't in
the loaded scene grid, so every conversion returns null and the call silently
aborts.

The proper upstream fix is splitting Rs2Player.getWorldLocation() into
getNavigationLocation() (current behavior, for pathfinder callers) and
getSceneLocation() (raw instance tile, for scene-anchor callers) and auditing
all ~30 callers. That is a larger refactor. These patches apply localized
workarounds at each downstream symptom so POH teleports work today.

Patches in this commit:

1. shortestpath/PohPanel.java
   - Add public static WorldPoint getExitPortalTile() accessor so other code
     can read the configured exit-portal tile without reaching into private
     fields.

2. shortestpath/ShortestPathPlugin.java (setTargets)
   - When the player is in a POH instance and an exit portal tile is
     configured, remap the pathfinder source from the template tile to the
     exit portal. Without this the pathfinder searches from a tile that has
     no registered POH transport edges and returns a pure walk path.

3. shortestpath/components/ExitTilePanel.java
   - detectTile() previously routed through Rs2GameObject.getGameObject(id),
     which is anchored on Rs2Player.getWorldLocation() and fails inside POH.
     Use Rs2TileObjectCache.query().withId(POH_EXIT_PORTAL).nearest() instead,
     matching the community fix pattern.

4. util/walker/Rs2Walker.java (setTarget, handleTransports, handleSpiritTree)
   - setTarget: same pathfinder-source remap as #2.
   - handleTransports: skip the origin-plane guard when the player is in a
     POH instance, since the player's template plane never matches the
     configured exit-portal plane.
   - handleTransports: for POH transports in a POH instance, pass the origin
     straight through instead of calling WorldPoint.toLocalInstance (which
     returns empty because the exit-portal tile isn't in the player's
     instance chunks).
   - handleSpiritTree: fall back to PohTeleports.getSpiritTree() when
     findObjectById returns null, so the POH spirit tree is reachable.

5. util/poh/PohTeleports.java (isInHouse, usePortalNexus)
   - isInHouse() now uses Rs2TileObjectCache.query().withId(POH_EXIT_PORTAL)
     instead of Rs2GameObject.getGameObject, so it works on large houses where
     the exit portal is outside render range of Rs2Player.getWorldLocation().
   - usePortalNexus() scans Scene.getTiles() directly for any PORTAL_IDS match
     instead of relying on the broken anchor lookup.

6. util/poh/data/NexusPortal.java
   - Expand PORTAL_IDS to cover all current Nexus object variants
     (POH_TELENEXUS_1/2/3, POH_NEXUS_4_AMULET, POH_NEXUS_5_AMULET, etc.).
     Without this, many POHs register zero Nexus transports.

7. util/gameobject/Rs2GameObject.java + util/tileobject/Rs2TileObjectModel.java
   - Anchor fallback: when localPointFromWorldSafe(Rs2Player.getWorldLocation())
     returns null (i.e. inside a POH), fall back to the player's raw
     LocalPoint. Touches every getGameObject anchor site that currently
     aborts early.
   - Distance gate in clickObject / Rs2TileObjectModel.click: when the target
     object is in the current scene, compare LocalPoint sceneX/sceneY instead
     of Rs2Player.getWorldLocation().distanceTo(object.getWorldLocation()),
     which returns Integer.MAX_VALUE across the POH template/instance
     coordinate boundary and falsely triggers the "not close enough, walking"
     branch.

Verification

End-to-end tested via a debug Hub plugin (pohteleportdebug) that enters the
POH via Construction cape and runs all four teleport types sequentially on a
fully configured Hosidius POH:

  - Portal Nexus -> Grand Exchange                                    OK
  - Spirit Tree -> Tree Gnome Village  (arrived 2 tiles from dest)    OK
  - Fairy Ring AJP -> Mudskipper Point (arrived 2 tiles from dest)    OK
  - Ornate Jewellery Box -> Edgeville Monastery                       OK

All four now succeed where every one of them previously hung or returned
early. Full symptom analysis and logs are in issue #1741.

Closes #1741
Updates DEPOSITBOX_INVENTORY_ITEM_CONTAINER_COMPONENT_ID from 12582935 to 12582936
to correctly target the inventory container. This resolves the issue where
Deposit-All (via invokeMenu 6) was failing by trying to interact with the quiver slot.
Updated script does not assemble CombatInterfaceSP.rs2asm
fix(poh): unblock Rs2Walker and POH teleport utilities inside POH instances
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 15, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 650b678f-9653-419f-9e69-646e7f5aedd6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

This PR updates the Microbot version from 2.4.0 to 2.5.0 and introduces security hardening for the agent server alongside pathfinding improvements. Security changes include auth token configuration and validation (enforcing loopback-only access, rejecting cross-origin requests, requiring token authentication), disabling the agent server plugin by default, and adding file-based token storage with restricted permissions. Pathfinding changes improve POH (Player-Owned House) handling by detecting configured exit portals and remapping pathfinder start positions in instances, plus expanding Nexus portal detection to additional object variants. Utility updates include fallback logic for world-to-local coordinate conversion, instance-aware distance calculations, transport handling refinements, and a deposit box widget component ID adjustment. Test coverage shifts from CORS preflight validation to cross-origin rejection and authentication enforcement scenarios.

Possibly related PRs

  • PR #1742 — Overlapping code-level changes to POH/shortest-path/walker classes (PohPanel, ShortestPathPlugin, ExitTilePanel, Rs2Walker, PohTeleports, NexusPortal, Rs2GameObject, Rs2TileObjectModel)
  • PR #1734 — Directly modifies the agent-server subsystem (AgentServerPlugin, AgentHandler, AgentServerConfig, and related handler tests)
  • PR #1743 — Updates the same Rs2DepositBox constant (DEPOSITBOX_INVENTORY_ITEM_CONTAINER_COMPONENT_ID from 12582935 to 12582936)
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the two main change areas: AgentServer security improvements and POH teleport/walker fixes in instances.
Description check ✅ Passed The description is clearly related to the changeset, detailing security enhancements, POH pathfinding fixes, and robustness improvements with specific references.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/agentserver/AgentServerPlugin.java`:
- Around line 123-131: The token file is currently shared globally causing
races; modify writeTokenFile(...) and deleteTokenFile() so they use a
per-instance name (e.g., include the server port or a generated UUID in the
filename) rather than the fixed "~/.microbot/agent-token", update the startup
sequence around server.start(), shutdownHook, and the log.info call to use and
display that per-instance Path (use the existing writeTokenFile(token) return
value to persist and log the exact path), and ensure deleteTokenFile() deletes
that specific per-instance file (pass or store the Path returned from
writeTokenFile so the shutdownHook calls deleteTokenFile(path) instead of
removing a global file).

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.java`:
- Around line 736-755: The remap overwrites start for every instanced scene;
change the condition so we only remap when the player is actually in their POH
instance by requiring either PohTeleports.isInHouse() or a sanity check that the
exitPortal is in the same instance as the player (e.g., compare exitPortal to
rawStart with a proximity check like exitPortal.distanceTo(rawStart) < 50).
Update the if that currently uses PohPanel.getExitPortalTile() and
client.getTopLevelWorldView().getScene().isInstance() to instead require
exitPortal != null && inInstance && (PohTeleports.isInHouse() ||
exitPortal.distanceTo(rawStart) < 50) before assigning start = exitPortal.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2GameObject.java`:
- Around line 672-677: Several POH fallback branches (e.g., where anchorLocal is
set) call Microbot.getClient().getLocalPlayer() and getLocalLocation() directly
from non-client threads; wrap these client-only accesses in a client-thread
lambda and return the LocalPoint back to the caller. For each occurrence (the
anchorLocal fallbacks in Rs2GameObject methods such as getGameObject and other
POH helpers), replace direct calls to
Microbot.getClient().getLocalPlayer().getLocalLocation() with a
Microbot.getClientThread().invoke(...) or runOnClientThread(...) that safely
fetches the LocalPoint and assigns it to anchorLocal (or returns null), then
continue using that returned value in the existing logic; follow the existing
correct pattern used around lines 1527 and 1886 to mirror error handling and
null checks.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/tileobject/Rs2TileObjectModel.java`:
- Around line 171-184: In click(String action) the direct access to
Microbot.getClient().getLocalPlayer() must be moved onto the client thread:
replace the inline playerLocal assignment with a call to
Microbot.getClientThread().invoke(...) (or runOnClientThread) that returns
Microbot.getClient().getLocalPlayer() != null ?
Microbot.getClient().getLocalPlayer().getLocalLocation() : null; keep
objectLocal = getLocalLocation() and the subsequent distance logic unchanged and
ensure you still fall back to
Rs2Player.getWorldLocation().distanceTo(getWorldLocation()) when playerLocal is
null.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java`:
- Around line 1368-1374: Use a consistent POH instance check in Rs2Walker by
replacing any uses of
Microbot.getClient().getTopLevelWorldView().getScene().isInstance() (found in
handleTransports) with Microbot.getClient().getTopLevelWorldView().isInstance()
so it matches the check used in setTarget; update the instance-checking
expression wherever handleTransports performs the scene-based check to use the
top-level WorldView isInstance() call instead.

In
`@runelite-client/src/test/java/net/runelite/client/plugins/microbot/agentserver/handler/ScriptHandlerTest.java`:
- Around line 115-125: In testAcceptsValidToken, replace the loose auth-negation
checks with a single explicit success assertion: capture conn.getResponseCode()
into a local variable and assert it equals the expected success status (e.g.,
200) instead of using assertNotEquals for 401 and 403; keep the
AgentHandler.setTokenSupplier setup/teardown and the use of
openConnection("/scripts") and conn.setRequestProperty("X-Agent-Token", ...)
intact so the test verifies that a valid token yields the expected success
status.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e58d390f-4826-45c3-96d8-1bb1b34e6c80

📥 Commits

Reviewing files that changed from the base of the PR and between a33d7f1 and e305f88.

📒 Files selected for processing (16)
  • gradle.properties
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agentserver/AgentServerConfig.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agentserver/AgentServerPlugin.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/agentserver/handler/AgentHandler.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/PohPanel.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/components/ExitTilePanel.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/depositbox/Rs2DepositBox.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2GameObject.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/PohTeleports.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/poh/data/NexusPortal.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/tileobject/Rs2TileObjectModel.java
  • runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java
  • runelite-client/src/test/java/net/runelite/client/plugins/microbot/agentserver/handler/LoginHandlerTest.java
  • runelite-client/src/test/java/net/runelite/client/plugins/microbot/agentserver/handler/ScreenshotHandlerTest.java
  • runelite-client/src/test/java/net/runelite/client/plugins/microbot/agentserver/handler/ScriptHandlerTest.java

Comment on lines +123 to +131
shutdownHook = new Thread(() -> {
stopServer();
deleteTokenFile();
}, "AgentServer-Shutdown");
Runtime.getRuntime().addShutdownHook(shutdownHook);
server.start();
log.info("Agent server started on port {} with {} endpoints", port, handlers.size());
Path tokenFile = writeTokenFile(token);
log.info("Agent server started on 127.0.0.1:{} with {} endpoints (auth enabled, token {})",
port, handlers.size(), tokenFile != null ? "at " + tokenFile : "file unavailable");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use a per-instance token file instead of one shared ~/.microbot/agent-token.

The server is per instance/per port, but the token file is global. If two clients run on different ports, the second startup overwrites the first client’s token file, and shutting either one down deletes the shared file for the other. That leaves the surviving server authenticated but no longer discoverable through the documented token handoff.

Possible direction
- Path file = dir.resolve("agent-token");
+ Path file = dir.resolve("agent-token-" + config.port());

Apply the same naming in deleteTokenFile() and in the startup log so each running server owns its own token file.

Also applies to: 158-205, 214-219

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/agentserver/AgentServerPlugin.java`
around lines 123 - 131, The token file is currently shared globally causing
races; modify writeTokenFile(...) and deleteTokenFile() so they use a
per-instance name (e.g., include the server port or a generated UUID in the
filename) rather than the fixed "~/.microbot/agent-token", update the startup
sequence around server.start(), shutdownHook, and the log.info call to use and
display that per-instance Path (use the existing writeTokenFile(token) return
value to persist and log the exact path), and ensure deleteTokenFile() deletes
that specific per-instance file (pass or store the Path returned from
writeTokenFile so the shutdownHook calls deleteTokenFile(path) instead of
removing a global file).

Comment on lines +736 to +755
WorldPoint rawStart = WorldPoint.fromLocalInstance(client, localPlayer.getLocalLocation());
// When the player is inside a POH instance, the raw instance-template tile
// (e.g. (1941,7052,3)) doesn't match any registered POH transport origin — the
// POH transports are keyed to PohPanel.instance.tilePanel.getTile() (the exit
// portal). Without this remap the pathfinder never considers any POH teleport
// and the walker tight-loops on null LocalPoint canvas-walks.
//
// We gate on "in an instance AND the POH panel has an exit-portal tile
// configured". PohTeleports.isInHouse() is too strict — it additionally
// requires POH_EXIT_PORTAL to be currently loaded in the scene, which fails
// on larger houses where the portal is out of render range.
WorldPoint exitPortal = PohPanel.getExitPortalTile();
boolean inInstance = client.getTopLevelWorldView().getScene().isInstance();
if (exitPortal != null && inInstance) {
Microbot.log("[ShortestPath] In POH instance — remapping pathfinder start "
+ rawStart + " -> exit portal " + exitPortal);
start = exitPortal;
} else {
start = rawStart;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Restrict this remap to actual POH instances.

This now rewrites start for every instanced scene as long as an exit tile is configured. In raids, boss instances, etc., the pathfinder will start from the saved POH portal tile instead of the player's real location, which will skew transport selection and route generation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/shortestpath/ShortestPathPlugin.java`
around lines 736 - 755, The remap overwrites start for every instanced scene;
change the condition so we only remap when the player is actually in their POH
instance by requiring either PohTeleports.isInHouse() or a sanity check that the
exitPortal is in the same instance as the player (e.g., compare exitPortal to
rawStart with a proximity check like exitPortal.distanceTo(rawStart) < 50).
Update the if that currently uses PohPanel.getExitPortalTile() and
client.getTopLevelWorldView().getScene().isInstance() to instead require
exitPortal != null && inInstance && (PohTeleports.isInHouse() ||
exitPortal.distanceTo(rawStart) < 50) before assigning start = exitPortal.

Comment on lines 672 to +677
if (anchorLocal == null) {
return null;
// POH fix: see Rs2GameObject.getGameObject(Predicate, WorldPoint, int).
if (Microbot.getClient() != null && Microbot.getClient().getLocalPlayer() != null) {
anchorLocal = Microbot.getClient().getLocalPlayer().getLocalLocation();
}
if (anchorLocal == null) return null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e

echo "Inspecting direct client-thread-sensitive accesses in Rs2GameObject.java"
rg -n -C2 'getLocalPlayer\(\)|getLocalLocation\(\)' \
  runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2GameObject.java

echo
echo "Locating the client-thread guardrail test and baseline referenced by repo learnings"
fd -i 'ClientThreadGuardrailTest.java' runelite-client/src/test/java
fd -i 'client-thread-guardrail-baseline.txt' runelite-client/src/test/resources

Repository: chsami/Microbot

Length of output: 15223


Wrap these POH fallback client-thread accesses in lambda blocks.

The POH fallback branches at lines 672–677, 728–735, 866–877, 928–934, 1055–1059, 1110–1114, 1243–1247, 1298–1302, 1431–1435, 1486–1490, and 1776–1790 call getLocalPlayer() and getLocalLocation() directly from utility methods. These are client-thread-only APIs and must be accessed only within Microbot.getClientThread().invoke(...) or .runOnClientThread(...) lambdas. The file already demonstrates this pattern correctly at lines 1527 and 1886. Fetch the fallback LocalPoint inside a client-thread lambda, then pass the result back to callers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/gameobject/Rs2GameObject.java`
around lines 672 - 677, Several POH fallback branches (e.g., where anchorLocal
is set) call Microbot.getClient().getLocalPlayer() and getLocalLocation()
directly from non-client threads; wrap these client-only accesses in a
client-thread lambda and return the LocalPoint back to the caller. For each
occurrence (the anchorLocal fallbacks in Rs2GameObject methods such as
getGameObject and other POH helpers), replace direct calls to
Microbot.getClient().getLocalPlayer().getLocalLocation() with a
Microbot.getClientThread().invoke(...) or runOnClientThread(...) that safely
fetches the LocalPoint and assigns it to anchorLocal (or returns null), then
continue using that returned value in the existing logic; follow the existing
correct pattern used around lines 1527 and 1886 to mirror error handling and
null checks.

Comment on lines 171 to +184
public boolean click(String action) {
if (Rs2Player.getWorldLocation().distanceTo(getWorldLocation()) > 51) {
// See Rs2GameObject.clickObject for rationale: use LocalPoint-based distance in POH /
// any instance because Rs2Player.getWorldLocation() returns an overworld-template tile.
net.runelite.api.coords.LocalPoint playerLocal = Microbot.getClient().getLocalPlayer() != null
? Microbot.getClient().getLocalPlayer().getLocalLocation() : null;
net.runelite.api.coords.LocalPoint objectLocal = getLocalLocation();
boolean tooFar;
if (playerLocal != null && objectLocal != null && objectLocal.isInScene()) {
int dx = playerLocal.getSceneX() - objectLocal.getSceneX();
int dy = playerLocal.getSceneY() - objectLocal.getSceneY();
tooFar = Math.max(Math.abs(dx), Math.abs(dy)) > 51;
} else {
tooFar = Rs2Player.getWorldLocation().distanceTo(getWorldLocation()) > 51;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/tileobject/Rs2TileObjectModel.java"

echo "Changed click() block:"
sed -n '171,184p' "$FILE"

echo
echo "Nearby client access vs. invoke wrappers:"
rg -n -C2 'getLocalPlayer\(|getLocalLocation\(|Microbot\.getClientThread\(\)\.(invoke|runOnClientThread)' "$FILE"

Repository: chsami/Microbot

Length of output: 3344


Wrap client-thread access in invoke() to match existing patterns in this class.

Lines 174–175 directly access Microbot.getClient().getLocalPlayer() outside a client-thread invoke block, inconsistent with the invocation pattern used at lines 96 and 154 elsewhere in this class. Move the playerLocal assignment into Microbot.getClientThread().invoke(...) to ensure safe access from script threads, following the guideline: Use Microbot.getClientThread().runOnClientThread() for all client state access from script threads.

Affected lines
net.runelite.api.coords.LocalPoint playerLocal = Microbot.getClient().getLocalPlayer() != null
        ? Microbot.getClient().getLocalPlayer().getLocalLocation() : null;

Should be wrapped in:

net.runelite.api.coords.LocalPoint playerLocal = Microbot.getClientThread().invoke(() -> {
    return Microbot.getClient().getLocalPlayer() != null
            ? Microbot.getClient().getLocalPlayer().getLocalLocation() : null;
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/tileobject/Rs2TileObjectModel.java`
around lines 171 - 184, In click(String action) the direct access to
Microbot.getClient().getLocalPlayer() must be moved onto the client thread:
replace the inline playerLocal assignment with a call to
Microbot.getClientThread().invoke(...) (or runOnClientThread) that returns
Microbot.getClient().getLocalPlayer() != null ?
Microbot.getClient().getLocalPlayer().getLocalLocation() : null; keep
objectLocal = getLocalLocation() and the subsequent distance logic unchanged and
ensure you still fall back to
Rs2Player.getWorldLocation().distanceTo(getWorldLocation()) when playerLocal is
null.

Comment on lines +1368 to +1374
if (Microbot.getClient().getTopLevelWorldView().isInstance()) {
WorldPoint exitPortal = net.runelite.client.plugins.microbot.shortestpath.PohPanel.getExitPortalTile();
if (exitPortal != null) {
Microbot.log("[Walker] In POH instance — remapping pathfinder start " + start
+ " -> exit portal " + exitPortal);
start = exitPortal;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent instance detection between setTarget and handleTransports.

The POH instance check uses two different approaches for the same purpose:

  • Line 1368: Microbot.getClient().getTopLevelWorldView().isInstance()
  • Line 1473: Microbot.getClient().getTopLevelWorldView().getScene().isInstance()

These could potentially yield different results in edge cases. For consistency, both POH-related checks should use the same approach.

Suggested fix for consistency
-        boolean inPohInstance = Microbot.getClient().getTopLevelWorldView().getScene().isInstance()
+        boolean inPohInstance = Microbot.getClient().getTopLevelWorldView().isInstance()
                 && net.runelite.client.plugins.microbot.shortestpath.PohPanel.getExitPortalTile() != null;

Also applies to: 1473-1474

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/walker/Rs2Walker.java`
around lines 1368 - 1374, Use a consistent POH instance check in Rs2Walker by
replacing any uses of
Microbot.getClient().getTopLevelWorldView().getScene().isInstance() (found in
handleTransports) with Microbot.getClient().getTopLevelWorldView().isInstance()
so it matches the check used in setTarget; update the instance-checking
expression wherever handleTransports performs the scene-based check to use the
top-level WorldView isInstance() call instead.

Comment on lines +115 to +125
public void testAcceptsValidToken() throws IOException {
AgentHandler.setTokenSupplier(() -> "expected-token");
try {
HttpURLConnection conn = openConnection("/scripts");
conn.setRequestMethod("GET");
conn.setRequestProperty("X-Agent-Token", "expected-token");
assertNotEquals(401, conn.getResponseCode());
assertNotEquals(403, conn.getResponseCode());
} finally {
AgentHandler.setTokenSupplier(null);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Assert the success code here, not just “not auth failure”.

This test still passes if /scripts regresses to 404 or 500, so it doesn't actually prove that a valid token allows the request through. Assert the expected success status once and reuse that value.

Suggested assertion tweak
-			assertNotEquals(401, conn.getResponseCode());
-			assertNotEquals(403, conn.getResponseCode());
+			int code = conn.getResponseCode();
+			assertEquals(200, code);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@runelite-client/src/test/java/net/runelite/client/plugins/microbot/agentserver/handler/ScriptHandlerTest.java`
around lines 115 - 125, In testAcceptsValidToken, replace the loose
auth-negation checks with a single explicit success assertion: capture
conn.getResponseCode() into a local variable and assert it equals the expected
success status (e.g., 200) instead of using assertNotEquals for 401 and 403;
keep the AgentHandler.setTokenSupplier setup/teardown and the use of
openConnection("/scripts") and conn.setRequestProperty("X-Agent-Token", ...)
intact so the test verifies that a valid token yields the expected success
status.

@chsami chsami merged commit 4f3c30e into main Apr 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants